Skip to content

Fix setting modules build type for tsh and tctl#51986

Merged
vapopov merged 4 commits intomasterfrom
vapopov/tools-build-fix-ldflags
Feb 11, 2025
Merged

Fix setting modules build type for tsh and tctl#51986
vapopov merged 4 commits intomasterfrom
vapopov/tools-build-fix-ldflags

Conversation

@vapopov
Copy link
Copy Markdown
Contributor

@vapopov vapopov commented Feb 8, 2025

I've postponed merging backports of #51732 until verify nightly build since I didn't find BuildType modification in scripts for the client tools tsh and tctl. I've verified on the latest builds:

teleport-v18.0.0-dev-nightly20250207-1-f186fb4-darwin-arm64-bin.tar.gz
teleport-ent-v18.0.0-dev-nightly20250207-1-f186fb4-darwin-arm64-bin.tar.gz

For client tools, modules.GetModules().BuildType() always returns "oss", causing a warning to always be displayed for community and enterprise builds and disabling updates until the base URL environment variable is set.

Related:

Tag build: https://github.com/gravitational/teleport.e/actions/runs/13212233201

Comment thread lib/autoupdate/tools/helper.go
@vapopov vapopov added the no-changelog Indicates that a PR does not require a changelog entry label Feb 8, 2025
@zmb3
Copy link
Copy Markdown
Collaborator

zmb3 commented Feb 8, 2025

There is no such thing as enterprise builds for tsh, tctl, and Teleport Connect. We distribute single build that works for all types of clusters.

It took over a year of thoughtful changes to make this happen. Please maintain this property.

@sclevine
Copy link
Copy Markdown
Member

sclevine commented Feb 10, 2025

@zmb3 Vadym is trying to prevent AGPL builds of tsh/tctl from auto-updating to community or enterprise edition builds hosted on the CDN.

I think we have a few options to solve this:

  • Ensure that all builds of tsh/tctl that we offer have their edition set to community instead of oss, and block implicit use of cdn.teleport.dev by oss builds.
  • Allow all builds (including AGPL builds) of tsh/tctl to automatically update using the community edition teleport tgz from cdn.teleport.dev. Unsure if this is permitted.
  • Disable auto-updates if the base URL defaults to cdn.teleport.dev and the connected proxy reports oss edition. This is how agent auto-updates works, but it may be less ideal for client tools that often switch between clusters.

Do you have a preference for which approach is taken? I agree this shouldn't involve introducing additional builds of the binaries.

@zmb3
Copy link
Copy Markdown
Collaborator

zmb3 commented Feb 10, 2025

1 or 3 sound okay to me. I would avoid option 2.

Comment thread lib/autoupdate/tools/helper.go
Comment thread lib/modules/modules.go
@vapopov
Copy link
Copy Markdown
Contributor Author

vapopov commented Feb 10, 2025

@zmb3 @sclevine these changes with TOOLS_LDFLAGS make the community build when GITHUB_REPOSITORY_OWNER is set, same for the enterprise

GITHUB_REPOSITORY_OWNER=gravitational make build/tctl build/tsh

from teleport.e repository

…uested by the `webapi/find` response or set via an environment variable.
Comment thread lib/autoupdate/tools/updater.go Outdated
@vapopov
Copy link
Copy Markdown
Contributor Author

vapopov commented Feb 11, 2025

Verified on https://github.com/gravitational/teleport.e/actions/runs/13252616358 build, for community and enterprise packages

@sclevine
Copy link
Copy Markdown
Member

Thanks @vapopov, looks good!

Merged via the queue into master with commit 3d6b26b Feb 11, 2025
@vapopov vapopov deleted the vapopov/tools-build-fix-ldflags branch February 11, 2025 03:03
vapopov added a commit that referenced this pull request Feb 11, 2025
* Fix setting modules for tsh and tctl

* Restore IsOSSBuild()

* Add different warning messages depending on whether the update is requested by the `webapi/find` response or set via an environment variable.

* Check only the build type of client tools
vapopov added a commit that referenced this pull request Feb 11, 2025
* Fix setting modules for tsh and tctl

* Restore IsOSSBuild()

* Add different warning messages depending on whether the update is requested by the `webapi/find` response or set via an environment variable.

* Check only the build type of client tools
vapopov added a commit that referenced this pull request Feb 11, 2025
* Fix setting modules for tsh and tctl

* Restore IsOSSBuild()

* Add different warning messages depending on whether the update is requested by the `webapi/find` response or set via an environment variable.

* Check only the build type of client tools
github-merge-queue Bot pushed a commit that referenced this pull request Feb 11, 2025
* Default BaseURL only for community / enterprise editions (#51732)

* Add requirement to set base URL for OSS builds
Fix progress bar for darwin platform

* Show warning only before update

* Move error to teleportPackageURLs

* Move error to teleportPackageURLs

* Fix linter

* Fix setting modules build type for tsh and tctl (#51986)

* Fix setting modules for tsh and tctl

* Restore IsOSSBuild()

* Add different warning messages depending on whether the update is requested by the `webapi/find` response or set via an environment variable.

* Check only the build type of client tools
github-merge-queue Bot pushed a commit that referenced this pull request Feb 11, 2025
* Add requirement to set base URL for OSS builds
Fix progress bar for darwin platform

* Show warning only before update

* Move error to teleportPackageURLs

* Move error to teleportPackageURLs

* Fix linter

* Fix setting modules build type for tsh and tctl (#51986)

* Fix setting modules for tsh and tctl

* Restore IsOSSBuild()

* Add different warning messages depending on whether the update is requested by the `webapi/find` response or set via an environment variable.

* Check only the build type of client tools
probakowski pushed a commit that referenced this pull request Feb 12, 2025
* Default BaseURL only for community / enterprise editions (#51732)

* Add requirement to set base URL for OSS builds
Fix progress bar for darwin platform

* Show warning only before update

* Move error to teleportPackageURLs

* Move error to teleportPackageURLs

* Fix linter

* Fix setting modules build type for tsh and tctl (#51986)

* Fix setting modules for tsh and tctl

* Restore IsOSSBuild()

* Add different warning messages depending on whether the update is requested by the `webapi/find` response or set via an environment variable.

* Check only the build type of client tools
carloscastrojumo pushed a commit to carloscastrojumo/teleport that referenced this pull request Feb 19, 2025
* Fix setting modules for tsh and tctl

* Restore IsOSSBuild()

* Add different warning messages depending on whether the update is requested by the `webapi/find` response or set via an environment variable.

* Check only the build type of client tools
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog Indicates that a PR does not require a changelog entry size/sm

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants